Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update to voxpupuli-test 5 #841

Merged
merged 5 commits into from
Jan 11, 2023
Merged

Update to voxpupuli-test 5 #841

merged 5 commits into from
Jan 11, 2023

Conversation

ekohl
Copy link
Member

@ekohl ekohl commented Jun 22, 2022

I'm still a bit unsure about the fact handling. https://puppet.com/docs/puppet/7/lang_facts_builtin_variables.html is a very useful resource, but I'm not entirely sure what the intended values should be and how to deal with it in various environments. See the TODOs inline.

Copy link
Member Author

@ekohl ekohl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alexjfisher @bastelfreak would you mind taking a look at these comments?


# lint:ignore:puppet_url_without_modules
$pluginsource = 'puppet:///plugins'
$pluginfactsource = 'puppet:///pluginfacts'
# lint:endignore
$classfile = '$statedir/classes.txt'
$syslogfacility = undef
$environment = $::environment
$environment = $server_facts['environment']
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While https://www.puppet.com/docs/puppet/7/lang_facts_builtin_variables.html#lang_facts_builtin_variables-server-facts doesn't mention it, $server_facts['environment'] is present when using puppet apply.

} else {
$puppetmaster = undef
}
$client_certname = $trusted['certname']
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is also present when using puppet apply

$puppetmaster = undef
}
$client_certname = $trusted['certname']
$puppetmaster = $server_facts['servername']
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is one I wonder about. puppetmaster is a global variable we set in Foreman's ENC, which allows you to change the puppetserver easily. Also, setting this outside of Foreman's ENC. For example, when using DNS lookups. Should we keep the old defined code/update it to getvar()?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I ended up changing it to getvar('puppetmaster'), to remain compatible.

@ekohl ekohl force-pushed the voxpupuli-test-5 branch 3 times, most recently from f2ada39 to e2d56df Compare January 10, 2023 16:53
Comment on lines +2 to +3
c.trusted_node_data = true
c.trusted_server_facts = true
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@@ -503,6 +496,7 @@
end

it 'should not sync the crl' do
pending("rspec-puppet always sets $server_facts['servername']")
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Global variables are unspecified and unreliable in where they come from.
Instead, built in variables like $facts, $trusted and $server_facts are
better sources.
@ekohl
Copy link
Member Author

ekohl commented Jan 11, 2023

No content change compared to the previous version, just extracted part of it to #841 and rebased it on top. Also added a link to the rspec-puppet GH issue in the code itself.

@ekohl ekohl merged commit d1e6b34 into theforeman:master Jan 11, 2023
@ekohl ekohl deleted the voxpupuli-test-5 branch January 11, 2023 16:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants